Skip metadata processing after sending local reply #16154
Conversation
|
Hi @GinYM, welcome and thank you for your contribution. We will try to review your Pull Request as quickly as possible. In the meantime, please take a look at the contribution guidelines if you have not done so already. |
|
/wait |
Sure, will do that. |
GinYM
force-pushed
the
early-stop
branch
2 times, most recently
from
b5085b1 to
9846ad2
Compare
| codec_client_ = makeHttpConnection(lookupPort("http")); | ||
| // Send a headers only request. | ||
| IntegrationStreamDecoderPtr response; | ||
| EXPECT_ENVOY_BUG( |
There was a problem hiding this comment.
I think this test is the same as the DownstreamProtocolIntegrationTest.ContinueAfterLocalReply and it does not test the change you have made. To test you change the decodeHeaders() of your test filer should return StopAll and then the decodeMetadata() should just have ASSERT(false); in it. I think the exception should work too, but it would be harder to diagnose than ASSERT.
And the test should just test for response from local reply. If it hits the ASSERT in the decodeMetadata then it would fail.
There was a problem hiding this comment.
Thanks for review. Your are right, I have added StopIteration in decoderHeaders and removed the EXPECT_ENVOY_BUG. Aslo replace throw with assert.
|
/wait |
|
/retest |
|
Retrying Azure Pipelines: |
|
LGTM, you will need to resolve merge conflict before I can merge. |
|
/wait |
Signed-off-by: Yiming Jin <[email protected]>
…ply. Signed-off-by: Yiming Jin <[email protected]>
Signed-off-by: Yiming Jin <[email protected]>
Done. |
|
/retest |
|
Retrying Azure Pipelines: |
|
/retest |
|
Retrying Azure Pipelines: |
Signed-off-by: Yiming Jin <[email protected]>
Signed-off-by: Yiming Jin [email protected]
Commit Message: Skip metadata processing after sending local reply
Fixes #15654